Skip to content

Lieutenant Fact#94

Merged
srueg merged 2 commits intomasterfrom
lieutenant-fact
May 14, 2020
Merged

Lieutenant Fact#94
srueg merged 2 commits intomasterfrom
lieutenant-fact

Conversation

@srueg
Copy link
Copy Markdown
Contributor

@srueg srueg commented May 14, 2020

@srueg srueg force-pushed the lieutenant-fact branch 2 times, most recently from 1089d21 to d62c714 Compare May 14, 2020 09:25
@srueg srueg requested review from corvus-ch and simu May 14, 2020 09:26
Copy link
Copy Markdown
Member

@simu simu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change makes sense overall. It's a bit ugly that we have to re-extract a Lieutenant fact from the list of configured classes in local mode, but I guess we don't set the Lieutenant instance anywhere in the target.yml parameters.

Comment thread commodore/cluster.py Outdated
@srueg srueg force-pushed the lieutenant-fact branch from d62c714 to 8c9c3f9 Compare May 14, 2020 10:53
@srueg srueg requested a review from simu May 14, 2020 10:53
@srueg
Copy link
Copy Markdown
Contributor Author

srueg commented May 14, 2020

Yes, I was thinking the same. I guess once we implement #93 we can rework the local mode as well.

Copy link
Copy Markdown
Member

@simu simu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now. I think a rework for local mode is definitely necessary, especially in light of #93.

Comment thread commodore/cluster.py
srueg added 2 commits May 14, 2020 17:01
Signed-off-by: Simon Rüegg <simon@rueggs.ch>
If the fact `lieutenant-instance` is set, add it to the classes.

Relates to projectsyn/lieutenant-api#52

Signed-off-by: Simon Rüegg <simon@rueggs.ch>
@srueg srueg force-pushed the lieutenant-fact branch from 8c9c3f9 to 19447d7 Compare May 14, 2020 15:03
@srueg srueg merged commit 0f1de5f into master May 14, 2020
@srueg srueg deleted the lieutenant-fact branch May 14, 2020 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants